feat: dev/prod node isolation — lockfile, --dev flag, network modes, production build flag#780
Open
feat: dev/prod node isolation — lockfile, --dev flag, network modes, production build flag#780
Conversation
…production build flag - Data directory lockfile (.ad4m-lock) using fs2 advisory locking - --dev flag: defaults to ~/.ad4m-dev, devnet, debug logging - --network-mode <mainnet|devnet|local> flag - production cargo feature flag (IS_PRODUCTION_BUILD) - CLI --data-path for multi-instance discovery - Devnet bootstrap seed (separate from mainnet) - Tests for lockfile, network mode, production flag, devnet seed
Contributor
📝 WalkthroughWalkthroughAdds CLI --data-path and dev/network-mode controls, integrates devnet seed bootstrapping, exposes NetworkMode and build-time globals, enforces single-instance execution via an advisory file lock, and updates executor startup flow to use the provided data path. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Startup as CLI Startup
participant FS as Filesystem
participant Executor as Executor (rust-executor)
CLI->>Startup: parse args (--data-path, --dev, --network-mode, --app-data-path)
Startup->>Startup: determine NetworkMode (flag or --dev)
Startup->>FS: resolve app_data_path (default ~/.ad4m-dev if --dev)
alt NetworkMode == Devnet and no seed provided
Startup->>FS: write devnet_seed.seed using DEVNET_JSON
end
Startup->>FS: open/create .ad4m-lock in app_data_path
FS->>Startup: try_lock_exclusive (fs2)
alt lock acquired
Startup->>Executor: call run(config with computed fields)
Executor->>Executor: prepare(), hold lock, init services
else lock failed
Startup->>CLI: print stderr and exit(1)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Devnet isolation comes from separate data directories and Holochain conductors (separate agent identity, separate DHT state), NOT from different language binaries. The devnet seed now uses the same real trusted agents, language addresses, and language-language bundle as mainnet so that dev nodes can properly install and run languages. Updated test to verify devnet uses real language addresses rather than asserting devnet != mainnet.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/startup.rs (1)
24-39:⚠️ Potential issue | 🟠 MajorDon't fall back to
12000from the path-specific lookup.This helper now powers
--data-path. On read failures it silently redirects to the default port, and Line 33 panics on malformed contents. That means a missing or stale~/.ad4m-dev/executor-portcan either crash the CLI or send it to the prod executor instead of the requested instance.Possible fix
pub fn get_executor_port_from_path(data_path: &std::path::Path) -> Result<u16> { let file_path = data_path.join("executor-port"); - let executor_port = std::fs::read_to_string(file_path.clone()).with_context(|| { + let executor_port = std::fs::read_to_string(&file_path).with_context(|| { format!( "Could not get executor port file `{}`!\nIs AD4M executor running?", file_path.display() ) - }); - match executor_port { - Ok(port) => Ok(port.trim().parse::<u16>().unwrap()), - Err(err) => { - println!("{}", err); - println!("Attempting to connect on default port 12000...\n"); - Ok(12000) - } - } + })?; + executor_port + .trim() + .parse::<u16>() + .with_context(|| format!("Invalid executor port in `{}`", file_path.display())) } pub fn get_executor_port() -> Result<u16> { - get_executor_port_from_path(&executor_data_path()) + match get_executor_port_from_path(&executor_data_path()) { + Ok(port) => Ok(port), + Err(err) => { + println!("{}", err); + println!("Attempting to connect on default port 12000...\n"); + Ok(12000) + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/startup.rs` around lines 24 - 39, get_executor_port_from_path currently swallows read errors and returns a default 12000 (and also unwraps parse results which can panic); instead make the function surface errors: on failing to read the executor-port file return Err with context (do not print and do not fallback to 12000), and when parsing the file contents handle parse errors by returning an Err with contextual message rather than unwrap; update get_executor_port_from_path to propagate the read error from std::fs::read_to_string (using with_context) and to parse via trim().parse::<u16>().with_context(...) so callers can decide to use a default if appropriate.
🧹 Nitpick comments (1)
rust-executor/src/dev_prod_tests.rs (1)
64-95: Make this seed regression test assert exact reuse.The PR contract is that devnet reuses mainnet's trusted agents and language-language bundle. This only checks that the bundle is non-empty, so a drifted bundle or trusted-agent list would still pass. Compare
trustedAgentsandlanguageLanguageBundledirectly againstMAINNET_JSONhere (or compare a digest for the bundle if you want smaller failure output).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/dev_prod_tests.rs` around lines 64 - 95, The test function test_devnet_seed_is_valid_and_uses_real_languages currently only checks that devnet's bundle is non-empty and that some language fields match, which allows drift; update the assertions to assert exact reuse of mainnet's trusted agents and language-language bundle by comparing mainnet["trustedAgents"] == devnet["trustedAgents"] and mainnet["languageLanguageBundle"] == devnet["languageLanguageBundle"] (or compute and compare a digest/sha256 of the bundle strings if you prefer smaller failure output); use the existing serde_json::Value variables mainnet and devnet to perform these exact-equality checks so the test fails on any drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/ad4m.rs`:
- Around line 77-79: The log command ignores the CLI-provided data_path and
always reads executor_data_path(); update the Domain::Log handling to use the
parsed Option<String> data_path (the same source used for executor-port) instead
of calling executor_data_path() directly — locate the Domain::Log match arm and
replace its use of executor_data_path().join("ad4m.log") with a path derived
from the provided data_path (falling back to executor_data_path() only when
data_path is None); ensure the same change is applied to the other occurrence
mentioned (around the other 120-125 block) so both log reads honor the
--data-path option.
In `@cli/src/main.rs`:
- Around line 283-313: The CLI still leaves app_data_path as None when
--network-mode devnet is used without --dev, causing the executor to use the
mainnet directory; fix by treating NetworkMode::Devnet like dev for default data
dir: if network_mode == NetworkMode::Devnet && app_data_path.is_none() set
app_data_path to the dev path (e.g., ~/.ad4m-dev) before writing the devnet seed
and before constructing Ad4mConfig/spawning the executor (update the logic
around the app_data_path assignment and where network_bootstrap_seed is computed
to use that value). Also add a regression test exercising `run --network-mode
devnet` without `--dev` to assert the created data dir and seed file are under
the devnet-specific directory. Reference symbols: app_data_path, network_mode,
NetworkMode::Devnet, network_bootstrap_seed, and Ad4mConfig/config.prepare().
In `@rust-executor/src/lib.rs`:
- Around line 198-220: The startup path prepares a mode-specific config but
later calls RuntimeService::init_global_instance with a hardcoded/mainnet seed
path, ignoring dev overrides; update the call in run() to pass
config.network_bootstrap_seed (cloned and expect'd) to
RuntimeService::init_global_instance so the runtime uses the configured
bootstrap seed (preserving mainnet behavior when set and honoring devnet
overrides). Reference RuntimeService::init_global_instance and
config.network_bootstrap_seed to locate and change the invocation.
---
Outside diff comments:
In `@cli/src/startup.rs`:
- Around line 24-39: get_executor_port_from_path currently swallows read errors
and returns a default 12000 (and also unwraps parse results which can panic);
instead make the function surface errors: on failing to read the executor-port
file return Err with context (do not print and do not fallback to 12000), and
when parsing the file contents handle parse errors by returning an Err with
contextual message rather than unwrap; update get_executor_port_from_path to
propagate the read error from std::fs::read_to_string (using with_context) and
to parse via trim().parse::<u16>().with_context(...) so callers can decide to
use a default if appropriate.
---
Nitpick comments:
In `@rust-executor/src/dev_prod_tests.rs`:
- Around line 64-95: The test function
test_devnet_seed_is_valid_and_uses_real_languages currently only checks that
devnet's bundle is non-empty and that some language fields match, which allows
drift; update the assertions to assert exact reuse of mainnet's trusted agents
and language-language bundle by comparing mainnet["trustedAgents"] ==
devnet["trustedAgents"] and mainnet["languageLanguageBundle"] ==
devnet["languageLanguageBundle"] (or compute and compare a digest/sha256 of the
bundle strings if you prefer smaller failure output); use the existing
serde_json::Value variables mainnet and devnet to perform these exact-equality
checks so the test fails on any drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0d4f9f3-6dbc-42f1-b02c-5cc0ebc1654b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
cli/src/ad4m.rscli/src/main.rscli/src/startup.rsrust-executor/Cargo.tomlrust-executor/src/config.rsrust-executor/src/dev_prod_tests.rsrust-executor/src/devnet_seed.jsonrust-executor/src/globals.rsrust-executor/src/lib.rs
Comment on lines
+77
to
+79
| /// Data directory to read executor-port from (default: ~/.ad4m) | ||
| #[arg(long)] | ||
| data_path: Option<String>, |
Contributor
There was a problem hiding this comment.
log still ignores the selected instance path.
--data-path now selects which executor directory to inspect for executor-port, but Domain::Log still reads executor_data_path().join("ad4m.log") at Lines 163-169. ad4m --data-path ~/.ad4m-dev log will therefore still show the default instance's log.
Also applies to: 120-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/ad4m.rs` around lines 77 - 79, The log command ignores the
CLI-provided data_path and always reads executor_data_path(); update the
Domain::Log handling to use the parsed Option<String> data_path (the same source
used for executor-port) instead of calling executor_data_path() directly —
locate the Domain::Log match arm and replace its use of
executor_data_path().join("ad4m.log") with a path derived from the provided
data_path (falling back to executor_data_path() only when data_path is None);
ensure the same change is applied to the other occurrence mentioned (around the
other 120-125 block) so both log reads honor the --data-path option.
Comment on lines
+283
to
+313
| // Apply --dev defaults | ||
| let app_data_path = if dev && app_data_path.is_none() { | ||
| let home = dirs::home_dir().expect("Could not get home directory"); | ||
| Some(home.join(".ad4m-dev").to_string_lossy().into_owned()) | ||
| } else { | ||
| app_data_path | ||
| }; | ||
|
|
||
| // Apply network mode to config | ||
| let (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url) = match network_mode { | ||
| NetworkMode::Local => { | ||
| (Some(true), Some(false), Some(false), Some(String::new()), Some(String::new())) | ||
| } | ||
| _ => (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url), | ||
| }; | ||
|
|
||
| // For devnet mode, write devnet seed instead of mainnet | ||
| let network_bootstrap_seed = if network_mode == NetworkMode::Devnet && network_bootstrap_seed.is_none() { | ||
| // Will be handled in config.prepare() — write devnet seed file | ||
| let data_path = app_data_path.clone().unwrap_or_else(|| { | ||
| let home = dirs::home_dir().expect("Could not get home directory"); | ||
| home.join(".ad4m").to_string_lossy().into_owned() | ||
| }); | ||
| std::fs::create_dir_all(&data_path).ok(); | ||
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | ||
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | ||
| .expect("Could not write devnet seed file"); | ||
| Some(seed_path.to_string_lossy().into_owned()) | ||
| } else { | ||
| network_bootstrap_seed | ||
| }; |
Contributor
There was a problem hiding this comment.
--network-mode devnet still falls back to the mainnet state directory.
When dev is false, Lines 284-289 leave app_data_path as None. Because Ad4mConfig does not carry network_mode (rust-executor/src/config.rs:99-130), the spawned executor still uses its normal default data dir, and Lines 302-305 also place the generated seed under ~/.ad4m. ad4m run --network-mode devnet therefore shares mainnet state unless the caller also adds --dev or --app-data-path, which breaks the isolation this flag is supposed to provide.
💡 Suggested fix
- let app_data_path = if dev && app_data_path.is_none() {
+ let app_data_path = if app_data_path.is_none()
+ && (dev || network_mode == NetworkMode::Devnet)
+ {
let home = dirs::home_dir().expect("Could not get home directory");
Some(home.join(".ad4m-dev").to_string_lossy().into_owned())
} else {
app_data_path
};
@@
- let data_path = app_data_path.clone().unwrap_or_else(|| {
- let home = dirs::home_dir().expect("Could not get home directory");
- home.join(".ad4m").to_string_lossy().into_owned()
- });
+ let data_path = app_data_path
+ .clone()
+ .expect("devnet should always resolve to an isolated data path");Please add a regression test for run --network-mode devnet without --dev.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Apply --dev defaults | |
| let app_data_path = if dev && app_data_path.is_none() { | |
| let home = dirs::home_dir().expect("Could not get home directory"); | |
| Some(home.join(".ad4m-dev").to_string_lossy().into_owned()) | |
| } else { | |
| app_data_path | |
| }; | |
| // Apply network mode to config | |
| let (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url) = match network_mode { | |
| NetworkMode::Local => { | |
| (Some(true), Some(false), Some(false), Some(String::new()), Some(String::new())) | |
| } | |
| _ => (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url), | |
| }; | |
| // For devnet mode, write devnet seed instead of mainnet | |
| let network_bootstrap_seed = if network_mode == NetworkMode::Devnet && network_bootstrap_seed.is_none() { | |
| // Will be handled in config.prepare() — write devnet seed file | |
| let data_path = app_data_path.clone().unwrap_or_else(|| { | |
| let home = dirs::home_dir().expect("Could not get home directory"); | |
| home.join(".ad4m").to_string_lossy().into_owned() | |
| }); | |
| std::fs::create_dir_all(&data_path).ok(); | |
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | |
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | |
| .expect("Could not write devnet seed file"); | |
| Some(seed_path.to_string_lossy().into_owned()) | |
| } else { | |
| network_bootstrap_seed | |
| }; | |
| // Apply --dev defaults | |
| let app_data_path = if app_data_path.is_none() | |
| && (dev || network_mode == NetworkMode::Devnet) | |
| { | |
| let home = dirs::home_dir().expect("Could not get home directory"); | |
| Some(home.join(".ad4m-dev").to_string_lossy().into_owned()) | |
| } else { | |
| app_data_path | |
| }; | |
| // Apply network mode to config | |
| let (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url) = match network_mode { | |
| NetworkMode::Local => { | |
| (Some(true), Some(false), Some(false), Some(String::new()), Some(String::new())) | |
| } | |
| _ => (hc_use_mdns, hc_use_bootstrap, hc_use_proxy, hc_proxy_url, hc_bootstrap_url), | |
| }; | |
| // For devnet mode, write devnet seed instead of mainnet | |
| let network_bootstrap_seed = if network_mode == NetworkMode::Devnet && network_bootstrap_seed.is_none() { | |
| // Will be handled in config.prepare() — write devnet seed file | |
| let data_path = app_data_path | |
| .clone() | |
| .expect("devnet should always resolve to an isolated data path"); | |
| std::fs::create_dir_all(&data_path).ok(); | |
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | |
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | |
| .expect("Could not write devnet seed file"); | |
| Some(seed_path.to_string_lossy().into_owned()) | |
| } else { | |
| network_bootstrap_seed | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/main.rs` around lines 283 - 313, The CLI still leaves app_data_path
as None when --network-mode devnet is used without --dev, causing the executor
to use the mainnet directory; fix by treating NetworkMode::Devnet like dev for
default data dir: if network_mode == NetworkMode::Devnet &&
app_data_path.is_none() set app_data_path to the dev path (e.g., ~/.ad4m-dev)
before writing the devnet seed and before constructing Ad4mConfig/spawning the
executor (update the logic around the app_data_path assignment and where
network_bootstrap_seed is computed to use that value). Also add a regression
test exercising `run --network-mode devnet` without `--dev` to assert the
created data dir and seed file are under the devnet-specific directory.
Reference symbols: app_data_path, network_mode, NetworkMode::Devnet,
network_bootstrap_seed, and Ad4mConfig/config.prepare().
Comment on lines
+198
to
+220
| // Prepare config defaults before lockfile acquisition | ||
| config.prepare(); | ||
|
|
||
| let data_path = config.app_data_path.clone().unwrap(); | ||
|
|
||
| // Acquire advisory lockfile on the data directory | ||
| let lock_path = std::path::PathBuf::from(&data_path).join(".ad4m-lock"); | ||
| std::fs::create_dir_all(&data_path).expect("Could not create data directory"); | ||
| let lock_file = std::fs::File::create(&lock_path).expect("Could not create lockfile"); | ||
| if lock_file.try_lock_exclusive().is_err() { | ||
| eprintln!( | ||
| "ERROR: Another AD4M executor is already running against this data directory: {}", | ||
| data_path | ||
| ); | ||
| std::process::exit(1); | ||
| } | ||
| // Keep lock_file alive for the entire process lifetime | ||
| // (leaking is intentional — OS releases lock on process exit) | ||
| let _lock_file = Box::leak(Box::new(lock_file)); | ||
|
|
||
| // Print startup banner | ||
| if !globals::IS_PRODUCTION_BUILD { | ||
| eprintln!("[dev build]"); |
Contributor
There was a problem hiding this comment.
Use the configured bootstrap seed throughout run().
This new dev/prod startup path prepares a mode-specific config, but the same function later calls RuntimeService::init_global_instance with <app_data_path>/mainnet_seed.seed at Lines 401-407. A devnet launch can therefore write devnet_seed.seed and still boot the runtime against the mainnet filename. Using config.network_bootstrap_seed here preserves current mainnet behavior while keeping devnet overrides intact.
Suggested follow-up
RuntimeService::init_global_instance(
config
.network_bootstrap_seed
.clone()
.expect("network_bootstrap_seed not set in Ad4mConfig"),
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-executor/src/lib.rs` around lines 198 - 220, The startup path prepares a
mode-specific config but later calls RuntimeService::init_global_instance with a
hardcoded/mainnet seed path, ignoring dev overrides; update the call in run() to
pass config.network_bootstrap_seed (cloned and expect'd) to
RuntimeService::init_global_instance so the runtime uses the configured
bootstrap seed (preserving mainnet behavior when set and honoring devnet
overrides). Reference RuntimeService::init_global_instance and
config.network_bootstrap_seed to locate and change the invocation.
The ad4m-executor binary (ad4m_executor.rs) has its own Run struct separate from main.rs. Added the same --dev and --network-mode flags with identical behavior: devnet seed writing, data path defaults, log level, and dev mode banner. Also fixed .cargo/config.toml linker path to use CommandLineTools ld-classic instead of hardcoded Xcode 15.4 path. Verified locally: prod and dev nodes run in parallel on separate ports (4000/14000) with separate data dirs, lockfiles prevent conflicting instances on the same data directory.
The CI runner (marvin) has Xcode 15.4, not CommandLineTools. Local dev builds should use RUSTFLAGS env override instead.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/ad4m_executor.rs`:
- Around line 185-190: NetworkMode::Local currently only clears bootstrap URLs
but doesn't override inbound flags, so hc_use_proxy and hc_use_local_proxy can
still be set when constructing Ad4mConfig; update the logic that handles
NetworkMode::Local (the branch setting hc_use_bootstrap, hc_use_local_proxy,
hc_use_mdns, hc_use_proxy, hc_proxy_url, hc_bootstrap_url and the similar block
later around lines 237-243) to explicitly force proxy discovery off and mDNS on:
set hc_use_proxy = false, hc_use_local_proxy = false, hc_use_mdns = true and
clear hc_proxy_url and hc_bootstrap_url before building Ad4mConfig so Local mode
is guaranteed to be mDNS-only regardless of incoming CLI flags.
- Around line 253-256: The call to std::fs::create_dir_all(&data_path).ok()
silently ignores errors causing the later std::fs::write(&seed_path, ...) to
panic without the real cause; change this to check/propagate the error (e.g.,
use std::fs::create_dir_all(&data_path).expect("Could not create data
directory") or propagate with ? and return a Result) so failures creating
data_path are reported immediately; keep the subsequent construction of
seed_path and the write of rust_executor::globals::DEVNET_JSON unchanged but
ensure you reference and handle/create errors for create_dir_all on data_path
before attempting to write seed_path.
- Around line 231-234: The code sets the dev data directory based on the boolean
flag dev, so when a user passes --network-mode devnet (parsed into network_mode)
but not --dev the app_data_path stays unset and falls back to production; update
the logic that assigns app_data_path to also check network_mode for the
dev/devnet value (in the same scope where app_data_path, dev and network_mode
are defined) so that either dev == true or network_mode == "devnet" (or the
equivalent enum/variant) will set app_data_path to ~/.ad4m-dev; apply the same
change where the later fallback logic (the block handling app_data_path
defaults) currently uses dev so it uses network_mode as well to avoid reusing
the production conductor state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+185
to
+190
| mut hc_use_bootstrap, | ||
| hc_use_local_proxy, | ||
| hc_use_mdns, | ||
| mut hc_use_mdns, | ||
| hc_use_proxy, | ||
| hc_proxy_url, | ||
| hc_bootstrap_url, | ||
| mut hc_proxy_url, | ||
| mut hc_bootstrap_url, |
Contributor
There was a problem hiding this comment.
Local mode doesn't fully force proxy discovery off.
NetworkMode::Local only disables bootstrap and clears URLs. If the caller also passes --hc-use-proxy or --hc-use-local-proxy, those values still flow into Ad4mConfig, so this mode is no longer guaranteed to be mDNS-only.
💡 Suggested change
@@
- hc_use_local_proxy,
+ mut hc_use_local_proxy,
@@
- hc_use_proxy,
+ mut hc_use_proxy,
@@
NetworkMode::Local => {
hc_use_mdns = Some(true);
hc_use_bootstrap = Some(false);
+ hc_use_proxy = Some(false);
+ hc_use_local_proxy = Some(false);
hc_proxy_url = Some(String::new());
hc_bootstrap_url = Some(String::new());
}Also applies to: 237-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/ad4m_executor.rs` around lines 185 - 190, NetworkMode::Local
currently only clears bootstrap URLs but doesn't override inbound flags, so
hc_use_proxy and hc_use_local_proxy can still be set when constructing
Ad4mConfig; update the logic that handles NetworkMode::Local (the branch setting
hc_use_bootstrap, hc_use_local_proxy, hc_use_mdns, hc_use_proxy, hc_proxy_url,
hc_bootstrap_url and the similar block later around lines 237-243) to explicitly
force proxy discovery off and mDNS on: set hc_use_proxy = false,
hc_use_local_proxy = false, hc_use_mdns = true and clear hc_proxy_url and
hc_bootstrap_url before building Ad4mConfig so Local mode is guaranteed to be
mDNS-only regardless of incoming CLI flags.
Comment on lines
+231
to
+234
| if dev && app_data_path.is_none() { | ||
| let home = dirs::home_dir().expect("Could not get home directory"); | ||
| app_data_path = Some(home.join(".ad4m-dev").to_string_lossy().into_owned()); | ||
| } |
Contributor
There was a problem hiding this comment.
--network-mode devnet still falls back to the production data directory.
These defaults are keyed off dev, not the parsed network_mode. Running with --network-mode devnet but without --dev leaves app_data_path unset, so Lines 249-252 still fall back to ~/.ad4m and reuse the production conductor state instead of the isolated dev directory.
💡 Suggested change
- // Apply --dev defaults
- if dev && app_data_path.is_none() {
+ // Apply dev/devnet defaults
+ if app_data_path.is_none() && (dev || network_mode == NetworkMode::Devnet) {
let home = dirs::home_dir().expect("Could not get home directory");
app_data_path = Some(home.join(".ad4m-dev").to_string_lossy().into_owned());
}Also applies to: 249-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/ad4m_executor.rs` around lines 231 - 234, The code sets the dev data
directory based on the boolean flag dev, so when a user passes --network-mode
devnet (parsed into network_mode) but not --dev the app_data_path stays unset
and falls back to production; update the logic that assigns app_data_path to
also check network_mode for the dev/devnet value (in the same scope where
app_data_path, dev and network_mode are defined) so that either dev == true or
network_mode == "devnet" (or the equivalent enum/variant) will set app_data_path
to ~/.ad4m-dev; apply the same change where the later fallback logic (the block
handling app_data_path defaults) currently uses dev so it uses network_mode as
well to avoid reusing the production conductor state.
Comment on lines
+253
to
+256
| std::fs::create_dir_all(&data_path).ok(); | ||
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | ||
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | ||
| .expect("Could not write devnet seed file"); |
Contributor
There was a problem hiding this comment.
Don't discard create_dir_all failures here.
create_dir_all(...).ok() hides the actual filesystem error and turns it into a later write panic. Failing fast on directory creation will make devnet seed setup failures much easier to diagnose.
💡 Suggested change
- std::fs::create_dir_all(&data_path).ok();
+ std::fs::create_dir_all(&data_path)
+ .expect("Could not create data directory for devnet seed");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| std::fs::create_dir_all(&data_path).ok(); | |
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | |
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | |
| .expect("Could not write devnet seed file"); | |
| std::fs::create_dir_all(&data_path) | |
| .expect("Could not create data directory for devnet seed"); | |
| let seed_path = std::path::PathBuf::from(&data_path).join("devnet_seed.seed"); | |
| std::fs::write(&seed_path, rust_executor::globals::DEVNET_JSON) | |
| .expect("Could not write devnet seed file"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/ad4m_executor.rs` around lines 253 - 256, The call to
std::fs::create_dir_all(&data_path).ok() silently ignores errors causing the
later std::fs::write(&seed_path, ...) to panic without the real cause; change
this to check/propagate the error (e.g., use
std::fs::create_dir_all(&data_path).expect("Could not create data directory") or
propagate with ? and return a Result) so failures creating data_path are
reported immediately; keep the subsequent construction of seed_path and the
write of rust_executor::globals::DEVNET_JSON unchanged but ensure you reference
and handle/create errors for create_dir_all on data_path before attempting to
write seed_path.
lucksus
approved these changes
Apr 2, 2026
Member
lucksus
left a comment
lucksus
left a comment
There was a problem hiding this comment.
Not ran/tested but looks good!
| use rust_executor::config::NetworkMode; | ||
|
|
||
| // Refuse --dev in production builds | ||
| if rust_executor::globals::IS_PRODUCTION_BUILD && dev { |
Member
There was a problem hiding this comment.
Why? I hardly use dev builds because of the 100GBs for cargo artefacts and no GPU accelleration. So I would want to use this feature with a release build.
Member
There was a problem hiding this comment.
Ah just read below that this does not equal release built, but there is a special production feature flag... ok.
lucksus
approved these changes
Apr 2, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust-executor/src/lib.rs (1)
297-297: Secondconfig.prepare()call is redundant but harmless.Since
prepare()is fully idempotent (all field assignments useif field.is_none()guards), this second call after line 200 is a no-op. Consider removing it for clarity, though it has no functional impact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/lib.rs` at line 297, Remove the redundant second call to config.prepare(): the Config::prepare() invocation is idempotent (uses if field.is_none() guards) so the later call after the initial prepare() is a no-op; locate the duplicate config.prepare() call in rust-executor::lib.rs and delete the second invocation to improve clarity (keep the original prepare() call where configuration is first finalized).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rust-executor/src/lib.rs`:
- Line 297: Remove the redundant second call to config.prepare(): the
Config::prepare() invocation is idempotent (uses if field.is_none() guards) so
the later call after the initial prepare() is a no-op; locate the duplicate
config.prepare() call in rust-executor::lib.rs and delete the second invocation
to improve clarity (keep the original prepare() call where configuration is
first finalized).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables running development and production AD4M nodes side-by-side on the same machine without conflicts.
Problem
Currently, running a dev build of AD4M conflicts with a production node:
~/.ad4m— SQLite, Holochain conductor, and language stores can corrupt~/.ad4m/executor-portand can't target a dev instanceChanges
1. Data Directory Lockfile
{data_path}/.ad4m-lockat startup2.
--devConvenience Flag~/.ad4m-dev, network to devnet, log level to debug⚠️ DEVELOPMENT MODE3.
--network-mode <mainnet|devnet|local>mainnet: production bootstrap (default, unchanged)devnet: separate data directory and Holochain conductor — dev node gets its own agent identity and DHT state while using the same real language binarieslocal: mDNS only, no external bootstrap4. Production Build Feature Flag
cargo build --features productionfor release builds[dev build]banner--devflag5. CLI
--data-pathfor Multi-Instance Discoveryexecutor-portfrom specified data directoryad4m --data-path ~/.ad4m-dev status6. Devnet Seed
How Isolation Works
The isolation is architectural, not at the language/network level:
~/.ad4m)~/.ad4m-dev)~/.ad4m~/.ad4m-devThis means a dev node can install and run the same languages, create test neighbourhoods, and exercise the full stack — without any risk of corrupting production data or polluting production neighbourhoods with buggy code.
Testing
Migration
--devfor isolated developmentSummary by CodeRabbit